Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[JENKINS-73695] BUG: Dashboard shows white space on certain width space >900px #9667

Merged
merged 5 commits into from
Sep 1, 2024

Conversation

scherler
Copy link
Contributor

@scherler scherler commented Aug 27, 2024

Problem:
image
After with the pr:
image

See JENKINS-73695.

Testing done

Proposed changelog entries

  • human-readable text

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@janfaracik @timja @basil

Before the changes are marked as ready-for-merge:

Maintainer checklist

@scherler scherler changed the title No space [JENKINS-73695] No space Aug 27, 2024
@scherler scherler changed the title [JENKINS-73695] No space [JENKINS-73695] Dashboard shows white space on certain width space >900px Aug 28, 2024
@scherler
Copy link
Contributor Author

@janfaracik @timja There is a different approach to solve the underlying problem which would mean to change war/src/main/scss/base/_breakpoints.scss from 900 to 1020 however this PR is more robust since the breakpoint is a bit more based on try and error and in case the table is extended by some plugins the underlying bug would be present.

@scherler scherler marked this pull request as ready for review August 28, 2024 10:43
@scherler scherler changed the title [JENKINS-73695] Dashboard shows white space on certain width space >900px [JENKINS-73695] BUG: Dashboard shows white space on certain width space >900px Aug 28, 2024
@janfaracik
Copy link
Contributor

Tried it and works fine 👍


One for the future but would be good to have the table be properly responsive in the future, shuffling/showing/hiding elements as necessary to fit the screen.

@timja timja added bug For changelog: Minor bug. Will be listed after features web-ui The PR includes WebUI changes which may need special expertise labels Aug 28, 2024
Signed-off-by: Thorsten Scherler <[email protected]>
@scherler
Copy link
Contributor Author

@janfaracik yeah I agree, but in the end I did not wanted to start this since I am not sure about the extensibility of the columns and whether or not we want to combine the media query with using columnExtensions I guess we would need a discussion to which columns we want drop and which to keep

@janfaracik
Copy link
Contributor

@janfaracik yeah I agree, but in the end I did not wanted to start this since I am not sure about the extensibility of the columns and whether or not we want to combine the media query with using columnExtensions I guess we would need a discussion to which columns we want drop and which to keep

Definitely - will definitely be a big job.

I'm conducting some research on that topic now actually - https://forms.gle/51uqa5yyPvknDETb9 - if you'd be free for filling it in :) Should take a couple minutes at most.

Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@timja
Copy link
Member

timja commented Aug 30, 2024

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 30, 2024
@timja timja merged commit 1008dcb into jenkinsci:master Sep 1, 2024
16 checks passed
@scherler scherler deleted the no_space branch September 2, 2024 09:23
@strangelookingnerd
Copy link
Contributor

strangelookingnerd commented Sep 5, 2024

@Artmorse @janfaracik @scherler

I suspect this change to have introduced a small regression, as there seems to be a "useless" horizontal scrollbar in the overviews:

grafik

This may have gone unnoticed as the scrollbar disapears when the list / table loses focus.

@scherler
Copy link
Contributor Author

scherler commented Sep 5, 2024

I cannot reproduce it in Chrome (the focus should be on the table in the screenshot)
image
However in FF I can see it

@scherler
Copy link
Contributor Author

scherler commented Sep 5, 2024

image
Reducing the width by 1% fixes the problem it seems

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For changelog: Minor bug. Will be listed after features ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants